Repository navigation
feat(runtime): wrapper lifetime diagnostics and released-object arguments - #558
Draft
edusperoni wants to merge 2 commits into
Draft
edusperoni wants to merge 2 commits into
edusperoni wants to merge 2 commits into
Conversation
…ents
A JS object whose ObjC wrapper was released (a resurrected holder's graph
can keep it reachable) used to reach native code as a DictionaryAdapter
standing in for the object, so UIKit received a dictionary where it
expected, for example, a UIColor. A method call on it asserted in Release.
Released ObjC wrappers now carry a ReleasedReason in internal field 0
(collected, released through __releaseNativeCounterpart, or teardown).
GetValue only reads Externals, so existing callers are unaffected. Where
an object is expected, the marker routes the value through
releasedObjectPolicy: an argument becomes nil (report) or throws a
ReferenceError (throw), a collection element becomes NSNull, a
JS-implemented method's return value becomes nil, and a method or
property call on it always throws a ReferenceError, also through a Proxy.
ns:runtime gains getWrapperLifetimeStats() (resurrections, collections,
explicit releases, released-object accesses, state-lost candidates and
confirmations, counted on cold paths in every build) and the
wrapperLifetimeWarnings key. With warnings on (default in debug builds,
or "wrapperLifetime": { "warnings": bool } in the app config), a
collected wrapper that carried JS properties while its object was still
retained elsewhere is checked again a runloop turn later through a weak
reference, and a still-alive object is reported once per class.
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
clang rejects bit tests on ObjC object pointers under -Werror (-Wdeprecated-objc-pointer-introspection), which broke the x86_64 simulator archive.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First of a stacked series that moves ObjC wrapper lifetime off resurrecting finalizers, adopted in tiers behind a mode flag. This PR changes no lifetime behaviour. It makes released wrappers safe to touch where they used to reach native code as the wrong object, and adds counters and warnings that show where an app relies on resurrection today.
Current behavior
Externalleft, so it falls through to aDictionaryAdapterstanding in for it. A production app hit this as-[DictionaryAdapter _resolvedMaterialWithTraitCollection:]: unrecognized selector, when a releasedUIColorreached UIKit.tns::Assertin Release.New behavior
Released wrappers are marked.
DisposeValueand__releaseNativeCounterpartleave aReleasedReasonSmi in internal field 0 of an ObjC object's wrapper: collected, released explicitly, or teardown. Proxies are unwrapped first.tns::GetValueonly readsExternals, so every existing caller still sees "no wrapper". Writing the Smi allocates nothing inside the finalizer drain.metadataprivate are still only cleared.Released wrappers never become a
DictionaryAdapter. They go through the existingreleasedObjectPolicy:"report"(default)"throw"Interop::WriteValue)nil, plus thereleasednativeaccesseventReferenceErrorInterop::ToObject, from adapter callbacks)NSNull, plus the eventNSNull, plus the event (nothing can be thrown there)ArgConverter::SetValue)nil, plus the eventnil, plus the eventArgConverter::Invoke, Proxy receivers included)ReferenceErrorReferenceErrornil. A method call on a released wrapper now throws a catchableReferenceErrorin Release instead of asserting.Counters.
ns:runtimegainsgetWrapperLifetimeStats(), with per-isolate counts of resurrections, collections, explicit releases, released-object accesses, and state-lost candidates and confirmations.setExternalSize/getExternalSize.State-lost warnings.
"wrapperLifetime": { "warnings": bool }and thens:runtimekeywrapperLifetimeWarningsoverride it.Documentation:
docs/ns-builtin-modules.md,docs/knowledge/v8-resurrecting-finalizers.md,types/ns-runtime.d.ts.Performance
The only additions on non-error paths:
DictionaryAdapterfallback.The state-lost check runs only while warnings are on, which is off by default in Release.
Release A/B against
main(95b52af), simulator, alternating runs, 5 per side, existing bench specs plus a new wrapper churn spec. Medians are in the table; run-to-run spread overlaps on every case:NSObject.alloc().init()(wrapper churn)dict.setValueForKey({a: 1}, "k")(dictionary fallback)NSArray.arrayWithArray([{a: 1}])(collection fallback)All other bench cases (native calls, properties, structs, NSData, blocks) are within ±3% with no direction.
Tests
WrapperLifetimeTests.jscovers:NsRuntimeTests.jshas the new export.main: 1856/0). ASan suite: 1863 passed, 0 failed, no sanitizer reports.Next in the stack
wrapperLifetime { mode, scope }(resurrectingdefault; scopeextendeddefault) and a lifetime strategy interface aroundInstances/ObjectManager, withresurrectingthe only implementation.rootedmode: wrappers rooted while native code owns their object, with no resurrection.canarymode, the portable model shared with the Android and Node-API runtimes.